Resolve $ref/$defs when converting JSON schema to Python types#1908
Resolve $ref/$defs when converting JSON schema to Python types#1908chakshu-dhannawat wants to merge 3 commits into
Conversation
schema_type_to_python did not handle $ref, so any nested model reference
(the shape Pydantic emits for nested models, e.g. {"$ref": "#/$defs/Address"})
fell through to Any. This silently dropped the nested structure when a raw
JSON schema was converted to a pydantic/typeddict/dataclass, which is the
path Gemini uses for JSON-schema output types.
Resolve $ref against the root $defs/definitions and thread it through the
recursion. A seen-set guards against reference cycles so self-referential
(recursive) models degrade the cyclic edge to Any instead of recursing
forever. Unknown refs also fall back to Any.
Adds regression tests covering nested refs across all three targets, a
dangling ref, and a recursive model.
ErenAta16
left a comment
There was a problem hiding this comment.
Checked this against the actual behavior on main first, then ran the fix. Both hold up.
On main, the bug is exactly as described: a nested pydantic model comes through as {"$ref": "#/$defs/Address"}, schema_type_to_python has no $ref branch, so it falls past every type check and returns Any. Confirmed by converting a Person schema with a nested Address ref, the address field on the resulting model came out as typing.Any, the whole nested structure gone.
With this PR applied, the same conversion resolves address to a real Address model with city: str. The threading of defs/seen through every recursive call site (type-array union members, array items, and all three of the object converters) looks complete, I didn't find a recursion path that drops them.
The part I most wanted to check was the cycle handling, since a self-referential model is the obvious way to make $ref resolution loop forever. Built a Node schema whose child field refs Node itself. It resolves without a RecursionError: the first level materializes as a real Node model, and the repeated ref (now in seen) degrades to Any, so child ends up Optional[Node]. That's the right call, you can't build an infinitely-nested Python type anyway, so breaking after one level and widening is the sensible degradation rather than crashing.
Two small things, both non-blocking:
- The
$refsplit takes the last path segment (ref.split("/")[-1]), which is right for the pydantic#/$defs/Nameshape. It won't resolve refs into external documents or nested$defspaths, but those aren't what pydantic emits, so scoping it to the local-$defscase is reasonable. Might be worth a one-line note in the docstring that only local$defs/definitionsrefs are handled. defsis read from the root schema only on the top-level call (if defs is None). That's correct for pydantic output, where all definitions sit at the root, just flagging it as an assumption in case a schema nests its own$defsdeeper.
Solid fix, and good instinct adding the cycle guard rather than leaving it as a latent stack overflow.
|
Thanks for the thorough review, especially checking the self-referential cycle case, that was the part I was most careful about too. Good call on the docstring. Added a note (1f9768e) that only local |
|
The docstring note covers it, and folding both points into one at the resolution site was the right call. They really are the same decision, and two separate notes would have implied two independent limitations. Nothing further from me on this one. |
What
schema_type_to_python(inoutlines/types/json_schema_utils.py) never handled$ref. Pydantic emits nested models as references ({"$ref": "#/$defs/Address"}) with the real schema under the root$defs, so any nested-model field fell through toAny. The nested structure was silently lost when a raw JSON schema was converted to a pydantic model / TypedDict / dataclass.This matters in practice because
JsonSchema.convert_to(schema, ["dataclass", "typeddict", "pydantic"])is exactly the path the Gemini backend uses when you pass a JSON schema as the output type, so nested objects were not being constrained.Repro (before)
Fix
$refagainst the root$defs/definitionsand thread it through the recursion (array items, type-list unions, and the object -> converter calls).seenset of ref names on the current path, so self-referential (recursive) models degrade the cyclic edge toAnyinstead of recursing forever. Without this a wrapped recursive model would hitRecursionError.Any(no crash), same as before.After the fix,
addressresolves to the rebuiltAddressmodel,tagstoList[Address], and the reconstructed model validates the nested structure end to end.Scope
I deliberately kept this to direct
$refresolution.Optional[NestedModel]is emitted asanyOf: [{$ref}, {"type": "null"}];anyOfis a separate pre-existing limitation this PR doesn't touch, so that specific case still widens toAny(unchanged behavior, not a regression).Tests
Added regression tests in
tests/types/test_json_schema_utils.pycovering nested$refacross pydantic/typeddict/dataclass, a dangling ref, and a recursive model. They fail on the current code and pass with the fix. Fulltests/types/suite is green;ruffandmypyclean.